Skip to content

feat: staking validators discovery#357

Open
MuncleUscles wants to merge 2 commits into
v0.40-devfrom
feat/validator-discovery
Open

feat: staking validators discovery#357
MuncleUscles wants to merge 2 commits into
v0.40-devfrom
feat/validator-discovery

Conversation

@MuncleUscles

@MuncleUscles MuncleUscles commented Jul 2, 2026

Copy link
Copy Markdown
Member

Tooling-wave: genlayer staking validators — lists the active set with stake/status/delegator-count for CLI-only delegator onboarding (the missing piece of the full CLI delegation lifecycle). Optional --explorer-url enrichment with graceful chain-only fallback. --json for agents. Mock-based command tests.

Summary by CodeRabbit

  • New Features

    • Added a new staking validators command with table or JSON output.
    • Added sorting by stake or uptime, plus optional explorer-backed performance details.
    • Included more validator status and stake information in command results.
  • Bug Fixes

    • Improved validator data handling so missing explorer data won’t block output.
    • Added support for more flexible explorer URL formats and response shapes.
  • Tests

    • Added coverage for the new command options and JSON output behavior.
    • Added tests for explorer enrichment and sorting.

@github-actions github-actions Bot changed the base branch from main to v0.40-dev July 2, 2026 22:13
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

This PR targeted main, which is only the default/static branch.

I retargeted it to v0.40-dev, the active development branch. Pushes to v0.40-dev automatically fast-forward main.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c03e5ced-a117-42a6-8472-da98ff21783c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Introduces a new staking validators CLI command implemented via ValidatorsAction, replacing the prior listValidators/StakingInfoAction wiring. Supports JSON/table output, sorting, and optional explorer-based performance/delegator enrichment. Adds corresponding CLI parsing and unit tests.

Changes

Staking validators command

Layer / File(s) Summary
CLI command wiring
src/commands/staking/index.ts
Replaces the StakingInfoAction-based validators command with ValidatorsAction.execute, updates description, and adds --json, --sort-by, --explorer-url options.
ValidatorsAction core execution flow
src/commands/staking/validators.ts
Defines ValidatorsOptions/internal row types and implements execute(): gathers validator sets, batches detail fetches, builds/sorts rows, and outputs JSON or table.
Explorer enrichment helpers
src/commands/staking/validators.ts
Adds explorer endpoint resolution, paginated fetching with timeout/abort, delegator-count enrichment, and metric normalization/merging.
Table rendering and formatting helpers
src/commands/staking/validators.ts
Adds JSON row mapping, cli-table3 printing, cell truncation/tagging, status coloring, and compact stake formatting.
Command and unit test coverage
tests/commands/staking.test.ts, tests/commands/stakingValidators.test.ts
Adds CLI flag-parsing tests for staking validators and unit tests covering chain-only JSON output plus explorer-enriched output with sorting.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested reviewers: cristiam86

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and matches the main change: adding staking validators discovery to the CLI.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/validator-discovery

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
tests/commands/staking.test.ts (1)

15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Relative import instead of @/* alias.

New import uses ../../src/commands/staking/validators rather than the configured @/* alias, per the repo's coding guidelines. This matches the pre-existing style of every other import in this file, so it's a minor, low-priority inconsistency rather than a new pattern.

As per coding guidelines, "Use @/* path alias to reference ./src/* and @@/tests/* path alias to reference ./tests/* in imports."

Also applies to: 28-28

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/commands/staking.test.ts` at line 15, The new import in staking.test.ts
uses a relative path instead of the repository’s path aliases. Update the import
for ValidatorsAction (and the other affected import in this file) to use the
configured `@/`* alias for src references, matching the existing import style and
the repo’s import guidelines.

Source: Coding guidelines

tests/commands/stakingValidators.test.ts (2)

91-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Good coverage of chain-only and explorer-enriched paths; consider adding a failure-fallback test.

Both happy-path cases (no explorer, and explorer with sorting) are well covered and align with the execute() flow shown in the upstream snippet. However, the PR objective emphasizes "graceful fallback to chain-only data" when explorer enrichment is attempted but fails/unreachable — that specific fallback path isn't tested here (only the "no explorerUrl provided" case is).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/commands/stakingValidators.test.ts` around lines 91 - 150, Add a test
for the failure fallback path in stakingValidators.execute(), where explorer
enrichment is attempted but the explorer request fails or is unreachable, and
verify it still emits chain-only JSON instead of throwing. Reuse the existing
setupAction() and JSON output assertions, but make fetchMock return a failed
response for the explorer endpoint(s) and assert the output matches the
chain-only shape with explorer.enabled false/null and validator
performance/delegator fields left unset or null.

1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use @/* alias for src import.

New test file imports ValidatorsAction via a relative path (../../src/commands/staking/validators) instead of the @/* alias mandated for .ts files.

As per coding guidelines, "Use @/* path alias to reference ./src/* and @@/tests/* path alias to reference ./tests/* in imports."

♻️ Proposed fix
-import {ValidatorsAction} from "../../src/commands/staking/validators";
+import {ValidatorsAction} from "`@/commands/staking/validators`";
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/commands/stakingValidators.test.ts` around lines 1 - 2, The test file
is importing ValidatorsAction with a relative src path instead of the required
alias. Update the import in stakingValidators.test.ts to use the `@/`* alias for
the src/commands/staking/validators module, keeping the rest of the test setup
unchanged and matching the project’s import conventions for .ts files.

Source: Coding guidelines

src/commands/staking/validators.ts (1)

345-364: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Guard the pagination loop against non-advancing pages.

The loop only terminates on validators.size < total or page <= 50. If the explorer reports a total larger than what its pages actually return (or returns empty/duplicate pages), validators.size stops growing while page keeps incrementing, issuing up to ~49 extra HTTP round-trips before the cap stops it. Break early when a page yields no new entries.

♻️ Proposed fix
           total = typeof response.total === "number" ? response.total : response.validators.length;

+          const before = validators.size;
           for (const item of response.validators as ExplorerValidatorSummary[]) {
             const address = item.validator_address || item.validatorAddress;
             if (!address) continue;
             validators.set(address.toLowerCase(), this.toExplorerPerformance(item));
           }

+          if (response.validators.length === 0 || validators.size === before) break;
           page += 1;
         } while (validators.size < total && page <= 50);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/staking/validators.ts` around lines 345 - 364, The pagination
loop in the validators fetch logic can keep advancing even when a page adds no
new validators, causing unnecessary requests; update the do/while in the
validators API flow to stop early when a fetched page yields zero new entries.
Use the existing validators map and the explorer parsing block in the fetch
routine that calls this.fetchJson and toExplorerPerformance, and add a
non-advancing-page check before incrementing page so the loop exits when results
are empty or fully duplicated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/staking/validators.ts`:
- Line 1: The import in validators.ts uses a relative cross-directory path to
BaseAction and should be switched to the `@/`* alias. Update the resolveNetwork
import to reference the same BaseAction symbol through the alias form used
elsewhere in src instead of ../../lib/actions/BaseAction, keeping the import
location and usage unchanged.

---

Nitpick comments:
In `@src/commands/staking/validators.ts`:
- Around line 345-364: The pagination loop in the validators fetch logic can
keep advancing even when a page adds no new validators, causing unnecessary
requests; update the do/while in the validators API flow to stop early when a
fetched page yields zero new entries. Use the existing validators map and the
explorer parsing block in the fetch routine that calls this.fetchJson and
toExplorerPerformance, and add a non-advancing-page check before incrementing
page so the loop exits when results are empty or fully duplicated.

In `@tests/commands/staking.test.ts`:
- Line 15: The new import in staking.test.ts uses a relative path instead of the
repository’s path aliases. Update the import for ValidatorsAction (and the other
affected import in this file) to use the configured `@/`* alias for src
references, matching the existing import style and the repo’s import guidelines.

In `@tests/commands/stakingValidators.test.ts`:
- Around line 91-150: Add a test for the failure fallback path in
stakingValidators.execute(), where explorer enrichment is attempted but the
explorer request fails or is unreachable, and verify it still emits chain-only
JSON instead of throwing. Reuse the existing setupAction() and JSON output
assertions, but make fetchMock return a failed response for the explorer
endpoint(s) and assert the output matches the chain-only shape with
explorer.enabled false/null and validator performance/delegator fields left
unset or null.
- Around line 1-2: The test file is importing ValidatorsAction with a relative
src path instead of the required alias. Update the import in
stakingValidators.test.ts to use the `@/`* alias for the
src/commands/staking/validators module, keeping the rest of the test setup
unchanged and matching the project’s import conventions for .ts files.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 46347f28-dae6-4448-8e98-7adb1ba8b799

📥 Commits

Reviewing files that changed from the base of the PR and between 2100255 and b6d2433.

📒 Files selected for processing (4)
  • src/commands/staking/index.ts
  • src/commands/staking/validators.ts
  • tests/commands/staking.test.ts
  • tests/commands/stakingValidators.test.ts

@@ -0,0 +1,608 @@
import {resolveNetwork} from "../../lib/actions/BaseAction";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use the @/* path alias for the cross-directory src/lib import.

../../lib/actions/BaseAction resolves to src/lib/actions/BaseAction and should be referenced via the alias.

♻️ Proposed fix
-import {resolveNetwork} from "../../lib/actions/BaseAction";
+import {resolveNetwork} from "`@/lib/actions/BaseAction`";

As per coding guidelines: "Use @/* path alias to reference ./src/* ... in imports".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import {resolveNetwork} from "../../lib/actions/BaseAction";
import {resolveNetwork} from "`@/lib/actions/BaseAction`";
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/staking/validators.ts` at line 1, The import in validators.ts
uses a relative cross-directory path to BaseAction and should be switched to the
`@/`* alias. Update the resolveNetwork import to reference the same BaseAction
symbol through the alias form used elsewhere in src instead of
../../lib/actions/BaseAction, keeping the import location and usage unchanged.

Source: Coding guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant